const-eval: always do mem-to-mem copies if there might be padding involved #148967

RalfJung · 2025-11-15T09:04:44Z

This is the final piece of the puzzle for #148470: when copying data of a type that has padding, always do a mem-to-mem copy, so that we always preserve the source padding exactly. That prevents rustc implementation choices from leaking into user-visible behavior.

This is technically a breaking change: the example at the top of #148470 no longer compiles with this. However, it seems very unlikely that anyone would have dependent on this. My main concern is not backwards compatibility, it is performance.

Fixes #148470

Actually that seems to be entirely fine, it even helps with some benchmarks! I guess the mem-to-mem codepath is actually faster than the scalar pair codepath for the copy itself. It can slow things down later since now we have to do everything bytewise, but that doesn't show up in our benchmarks and might not be very relevant after all (in particular, it only affects types with padding, so the rather common wide pointers still always use the efficient scalar representation).

So that would be my proposal to for resolving this issue then: to make const-eval behavior consistent, we always copy the padding from the source to the target. IOW, potentially pre-existing provenance in the target always gets overwritten (that part is already in #148259), and potentially existing provenance in padding in the source always gets carried over (that's #148967). If there's provenance elsewhere in the source our existing handling is fine:

If it's in an integer, that's UB during const-eval so we can do whatever.

If it's in a pointer, the the fragments must combine back together to a pointer or else we have UB.

If it's in a union we just carry it over unchanged.

@traviscross we should check that this special const-eval-only UB is properly reflected in the reference. Currently we have this but that only talks about int2ptr, not about invalid pointer fragments at pointer type. I also wonder if this shouldn't rather be part of "invalid values" to make it clear that this applies recursively inside fields as well.
EDIT: Reference PR is up at rust-lang/reference#2091.

Originally posted by @RalfJung in #148470

Worth noting that this does not resolve the concerns @theemathas had about -Zextra-const-ub-checks sometimes causing more code to compile. Specifically, with that flag, the behavior changes to "potentially existing provenance in padding in the source never gets carried over". However, it's a nightly-only flag (used by Miri) so while the behavior is odd, I don't think this is a problem.

Originally posted by @RalfJung in #148470

rustbot · 2025-11-15T09:04:47Z

Some changes occurred to the CTFE machinery

cc @RalfJung, @oli-obk, @lcnr

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

rustbot · 2025-11-15T09:04:49Z

r? @JonathanBrouwer

rustbot has assigned @JonathanBrouwer.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

RalfJung · 2025-11-15T09:05:19Z

@bors try
@rust-timer queue

…try> const-eval: always do mem-to-mem copies if there might be padding involved

rust-bors · 2025-11-15T11:23:40Z

☀️ Try build successful (CI)
Build commit: 78c81ee (78c81ee3917a99dcff6e2e6822800f0492c415c3, parent: 733108b6d4acaa93fe26ae281ea305aacd6aac4e)

rust-timer · 2025-11-15T12:42:56Z

Finished benchmarking commit (78c81ee): comparison URL.

Overall result: ❌✅ regressions and improvements - please read the text below

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @rustbot label: +perf-regression-triaged. If not, please fix the regressions and do another perf run. If its results are neutral or positive, the label will be automatically removed.

@bors rollup=never
@rustbot label: -S-waiting-on-perf +perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.2%	[0.0%, 0.3%]	7
Improvements ✅ (primary)	-2.8%	[-2.8%, -2.8%]	1
Improvements ✅ (secondary)	-0.4%	[-0.5%, -0.2%]	12
All ❌✅ (primary)	-2.8%	[-2.8%, -2.8%]	1

Max RSS (memory usage)

Results (primary -3.2%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-3.2%	[-3.2%, -3.2%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-3.2%	[-3.2%, -3.2%]	1

Cycles

Results (primary -2.7%, secondary -9.4%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-2.7%	[-2.7%, -2.7%]	1
Improvements ✅ (secondary)	-9.4%	[-16.0%, -2.8%]	2
All ❌✅ (primary)	-2.7%	[-2.7%, -2.7%]	1

Binary size

Results (primary -1.1%, secondary 0.0%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.0%	[0.0%, 0.0%]	1
Improvements ✅ (primary)	-1.1%	[-1.1%, -1.1%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-1.1%	[-1.1%, -1.1%]	1

Bootstrap: 472.272s -> 472.014s (-0.05%)
Artifact size: 388.64 MiB -> 388.68 MiB (0.01%)

RalfJung · 2025-11-15T13:07:07Z

Uh okay I guess this is actually good for perf.^^ At least for the benchmarks we have. The copy apparently gets a little cheaper, but we force more things to use the less efficient in-memory representation. The latter just does not seem to matter in our benchmarks. Just to be safe: @craterbot check

craterbot · 2025-11-15T13:07:37Z

👌 Experiment pr-148967 created and queued.
🤖 Automatically detected try build 78c81ee
⚠️ Try build based on commit c4acb77, but latest commit is 01194d7. Did you forget to make a new try build?
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

rustbot · 2025-11-16T10:30:01Z

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

theemathas · 2025-11-17T04:25:33Z

Most of the performance regressions are from the coercions benchmark. All it does is create an array of a large number of string literals in const. Why did this benchmark's performance regress? There is no padding involved in any of the types.

saethlin · 2025-11-17T04:48:04Z

That "regression" is between 0.15% and 0.06%. Since the effect is so miniscule my guess is that's the overhead of the new check itself.

However, it would be bad science to give this level of attention to the regressions while ignoring the improvements. A real understanding should be able to explain both.

The juice isn't worth the squeeze.

RalfJung · 2025-11-17T08:06:03Z

Since the effect is so miniscule my guess is that's the overhead of the new check itself.

Yeah, that's my guess too. Maybe we could have a fast-path for reference types as we know those never have padding.

RalfJung · 2025-11-17T08:30:32Z

@bors try
@rust-timer queue

…try> const-eval: always do mem-to-mem copies if there might be padding involved

rust-bors · 2025-11-17T10:48:20Z

☀️ Try build successful (CI)
Build commit: 1a91d48 (1a91d48d6a8faaba3ee57217f255dea1f9dfa30e, parent: 89fe96197d232f86d733566df31c6dcebd1750da)

rust-timer · 2025-11-17T12:07:30Z

Finished benchmarking commit (1a91d48): comparison URL.

Overall result: ❌✅ regressions and improvements - please read the text below

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @rustbot label: +perf-regression-triaged. If not, please fix the regressions and do another perf run. If its results are neutral or positive, the label will be automatically removed.

@bors rollup=never
@rustbot label: -S-waiting-on-perf +perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.3%	[0.2%, 0.3%]	8
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.3%	[-0.5%, -0.2%]	12
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

This benchmark run did not return any relevant results for this metric.

Cycles

Results (secondary -3.0%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.0%	[-3.0%, -3.0%]	1
All ❌✅ (primary)	-	-	0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 476.458s -> 473.043s (-0.72%)
Artifact size: 388.72 MiB -> 388.74 MiB (0.01%)

RalfJung · 2025-11-17T12:54:04Z

This does look slightly better, but not by much. The extra check we add is utterly trivial now when the type is a wide pointer, not sure why that would show up at all -- maybe LLVM is just having a harder time optimizing this code now.

JonathanBrouwer

Code looks good to me with one nit/question.
We need to wait for crater & lang team

View changes since this review

compiler/rustc_const_eval/src/interpret/place.rs

…olved

craterbot · 2025-11-19T09:50:35Z

🚧 Experiment pr-148967 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

RalfJung · 2025-11-19T20:32:19Z

I just realized this has the interesting consequence of making the following code UB according to Miri:

#![feature(core_intrinsics)]
#![feature(custom_mir)]

use std::intrinsics::mir::*;

#[custom_mir(dialect = "runtime", phase = "optimized")]
fn test(x: (i64, i8)) {
    mir! {
        {
            x = x;
            Return()
        }
    }
}

fn main() {
    test((0, 1));
}

I think that's fine, many such assignments where LHS and RHS overlap are already UB -- but ScalarPair types have been exempt so far. However this means we have to update the docs for which MIR assignments allow the LHS and RHS to overlap, and which do not.

(Note that this only affects MIR-level assignments. In the surface language obviously arbitrary overlap is allowed; MIR building introduces copies to avoid UB.)

craterbot · 2025-11-21T07:00:51Z

🎉 Experiment pr-148967 is completed!
📊 6 regressed and 2 fixed (737827 total)
📊 1953 spurious results on the retry-regessed-list.txt, consider a retry¹ if this is a significant amount.
📰 Open the summary report.

⚠️ If you notice any spurious failure please add them to the denylist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

re-run the experiment with crates=https://crater-reports.s3.amazonaws.com/pr-148967/retry-regressed-list.txt ↩

RalfJung · 2025-11-21T07:30:27Z

2k spurious results seems like a lot
@craterbot check crates=https://crater-reports.s3.amazonaws.com/pr-148967/retry-regressed-list.txt p=1

craterbot · 2025-11-21T07:30:38Z

👌 Experiment pr-148967-1 created and queued.
🤖 Automatically detected try build 1a91d48
⚠️ Try build based on commit 4ba01da, but latest commit is 4a3e937. Did you forget to make a new try build?
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

RalfJung · 2025-11-21T07:34:31Z

In terms of those 6 regressions

There's an ICE that seems unrelated ("no resolution for an import")
"unable to start container"
"unable to get packages from source"
A build failure in a C/C++ dependency

So, those all seem spurious too.

theemathas · 2025-11-21T07:58:42Z

The "no resolution for an import" one is #147966

craterbot · 2025-11-22T05:52:38Z

🚧 Experiment pr-148967-1 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 15, 2025

rustbot assigned JonathanBrouwer Nov 15, 2025